[Review] Review of #1819 #1837
[Review] Review of #1819 #1837
Conversation
Olshansk
changed the base branch from
main
to
experiment/eager-optimistic-metrics
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive refactoring of the relay miner's request processing pipeline, introducing a new mining supervisor architecture and performance tracking system. The changes transition from a single-threaded relay processing model to a more scalable architecture with dedicated worker pools and sophisticated performance monitoring.
- Introduces
RelayMiningSupervisorwith configurable worker pools for delayed validation and rewardability checks - Implements a new performance tracking system using buffered span recording for detailed timing analysis
- Refactors session management to use lock-free concurrent data structures (
xsync.Map)
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/relayer/proxy/sync.go |
Major refactoring of sync request handling with new supervisor integration and performance tracking |
pkg/relayer/proxy/mining_supervisor.go |
New mining supervisor implementation with worker pool and session cache management |
pkg/relayer/performance_tracker.go |
New performance tracking system with span-based timing and buffered metrics |
pkg/relayer/proxy/relay_meter.go |
Converted to use lock-free concurrent data structures and added relay cost memoization |
pkg/relayer/metrics.go |
Added new mining queue metrics and improved instruction timing infrastructure |
pkg/relayer/config/types.go |
Added mining supervisor configuration structures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| "context" | ||
| "strconv" | ||
|
|
||
| //"sync" |
There was a problem hiding this comment.
Remove commented-out import. The sync package import has been removed but the comment should be deleted.
| //"sync" |
| "context" | ||
| "fmt" | ||
|
|
||
| //"sync" |
There was a problem hiding this comment.
Remove commented-out sync import and mutex-related code. These have been replaced with concurrent data structures but the old implementations should be deleted.
jorgecuesta
left a comment
jorgecuesta
left a comment
There was a problem hiding this comment.
LGTM, most of them are rename and comments
Olshansk
merged commit 7a9c588
into
experiment/eager-optimistic-metrics
Changes to accompany review #2 of #1819